-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[feature] Rancher UI dashboard commands #474
base: master
Are you sure you want to change the base?
Conversation
@@ -838,3 +880,47 @@ const checkRancherRCDepsTemplate = `{{- define "componentsFile" -}} | |||
* {{ .Content }} ({{ .File }}, line {{ .Line }}) | |||
{{- end}} | |||
{{ end }}` | |||
|
|||
const updateDashboardReferencesScript = `#!/bin/bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you comment this explaining what this logic does?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, I also thought that it was missing some comments explaining a few of those lines.
Added a few comments explaining most of the steps in this script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this a script taken from somewhere else or you wrote it? If it's ECM code, please update this to be POSIX complaint and use /bin/sh
. Run this through shellcheck to catch any kind of issues.
Auth *Auth `json:"auth"` | ||
User *User `json:"user"` | ||
K3s *K3s `json:"k3s"` | ||
UI *UI `json:"ui"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this code needs to know about the UI repo unless there's code to check that the tag was created from the GitHub action when a tag is created in the dashboard repo.
UIRepoName string `json:"ui_repo_name"` | ||
PreviousTag string `json:"previous_tag"` | ||
ReleaseBranch string `json:"release_branch" validate:"required"` | ||
DryRun bool `json:"dry_run"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Despite using dry_run
as a config in other places, I think using the global flag defined in cmd/release/cmd/root.go
is way more convenient and better and easier to check if it's on or not
Tag string | ||
RancherUpstreamURL string `json:"rancher_upstream_url" validate:"required"` | ||
RancherReleaseBranch string `json:"rancher_release_branch" validate:"required"` | ||
DryRun bool `json:"dry_run"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as before, but adding that I'll definitely make a PR updating the rancher cmd
cd5900c
to
453d7b5
Compare
release/release.go
Outdated
@@ -853,3 +913,17 @@ As always, we welcome and appreciate feedback from our community of users. Pleas | |||
- [Check out our documentation](https://rancher.com/docs/k3s/latest/en/) for guidance on how to get started or to dive deep into K3s. | |||
- [Read how you can contribute here](https://github.com/rancher/k3s/blob/master/CONTRIBUTING.md) | |||
{{ end }}` | |||
|
|||
const uiReleaseNoteTemplate = ` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure about this, but shouldn't these constants for templates be defined at the beginning of the file?
Use: "dashboard [ga,rc] [version]", | ||
Short: "Tag dashboard releases", | ||
RunE: func(cmd *cobra.Command, args []string) error { | ||
if len(args) < 2 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing here, see about doing validations on the standard Validation Arguments function of cobra like:
https://github.com/rancher/ecm-distro-tools/blob/master/cmd/release/cmd/push.go#L47
dashboardImagesBaseURL = "https://github.com/" + dashboardOrg + "/" + dashboardRepo + "/releases" | ||
) | ||
|
||
func CreateRelease(ctx context.Context, client *github.Client, r *ecmConfig.DashboardRelease, opts *repository.CreateReleaseOpts, rc bool) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we get some documentation on public functions?
) | ||
|
||
func CreateRelease(ctx context.Context, client *github.Client, r *ecmConfig.UIRelease, opts *repository.CreateReleaseOpts, rc bool) error { | ||
fmt.Println("validating tag") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not opposed to logging here.
However, I remember some reviews I got telling me not to log anything at this level unless using the debug
flag.
Use: "dashboard [version]", | ||
Short: "Update Rancher's Dashboard and UI references and create a PR", | ||
RunE: func(cmd *cobra.Command, args []string) error { | ||
if len(args) < 1 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's try to use the standard ValidationArgs function of cobra for validating args?
https://github.com/rancher/ecm-distro-tools/blob/master/cmd/release/cmd/push.go#L47
Commands added
$ release [repo] release-notes
Added support for
repo
:ui
dashboard
Command examples:
$ release generate ui release-notes --milestone v2.9.1 --prev-milestone v2.9.0
$ release generate dashboard release-notes --milestone v2.9.1 --prev-milestone v2.9.0
This command will generate and output the release notes, based on two milestones.
$ release [repo] [ga,ra] [version]
Added support for
repo
:ui
dashboard
$ release tag dashboard rc v2.9.1
tags https://github.com/rafaelbreno/rancher-dashboard/releases/tag/v2.9.1-rc1$ release tag dashboard ga v2.9.1
tags https://github.com/rafaelbreno/rancher-dashboard/releases/tag/v2.9.0$ release update rancher dashboard [version]
This command is ran in the
rancher/rancher
fork clone, where it will run a script that updates the two necessary fields, and opens a PR againstrancher/rancher
.Command examples:
$ release update rancher dashboard v2.9.1-rc8
opens Bump Dashboard tov2.9.1-rc8
rancher#46947